Changes to solve issue #498: Replacing timer implementation #567
Conversation
| long long int totalDuration = timers[timerName].count(); | ||
| // Converting microseconds to seconds | ||
| long long int totalDurationSec = totalDuration / 1e6; | ||
| long long int totalDurationMicroSec = totalDuration % 1000000; |
There was a problem hiding this comment.
Why not use std::chrono::microseconds::rep here? That would avoid any casting issues.
There was a problem hiding this comment.
Converted all the values to std::chrono::duration, to avoid casting issues.
|
Hi there, Thanks for this nice contribution. I only had a few minor comments. Also it seems like the Windows build failed; could you take a look into this please? Once those things are handled I'll be happy to merge this in. If you like, please feel free to add your name to the list of contributors in |
|
Hi, I made the required changes, but the code failed on some tests that are not affected by the changes. I did not find anything because of which the CoverTreeAlternateMetricTest and SequenceClassificationTest tests failed. |
|
Those are random test failures; many of the mlpack tests are probabilistic and it is hard to keep all of them from failing... Do you want to add your name/email to core.hpp and COPYRIGHT.txt? This looks good to merge otherwise. |
|
I will add my name once I contribute a bit more to mlpack. For now you can merge the branch. |
|
I like the change to |
Changes to solve issue #498: Replacing timer implementation
|
All the changes look good. I will make sure the lines are shorter than 80 characters next time. Also, I had made a habit of not using namespaces, I will surely use the using namespace line whenever necessary. Thanks for changes. |
|
Sounds good. There are only a few places in mlpack where we can use using directives; in this case, timers.cpp is a header file, so we can use using to make things shorter. But most other things are header files, so we can't, and the long names end up being necessary... |
The commit includes changes for replacement of timers code with std::chrono library of C++11. Instead of 2 maps, we now use 3 maps. The extra map stores the starting time_point of the timer and rest the timers map stores the duration of microseconds elapsed in the timer. Since, this works cross-platform, the unnecessary codes have been removed.